Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tests: enable higher concurrency and adjust tests with outlier runtime #4904

Merged
merged 3 commits into from
Aug 8, 2023

Conversation

jcsp
Copy link
Collaborator

@jcsp jcsp commented Aug 4, 2023

Problem

I spent a few minutes seeing how fast I could get our regression test suite to run on my workstation, for when I want to run a "did I break anything?" smoke test before pushing to CI.

  • Test runtime was dominated by a couple of tests that run for longer than all the others take together
  • Test concurrency was limited to <16 by the ports-per-worker setting

There's no "right answer" for how long a test should
be, but as a rule of thumb, no one test should run
for much longer than the time it takes to run all the
other tests together.

Summary of changes

  • Make the ports per worker setting dynamic depending on worker count
  • Modify the longest running tests to run for a shorter time (test_duplicate_layers which uses a pgbench runtime) or fewer iterations (test_restarts_frequent_checkpoints).

Checklist before requesting a review

  • I have performed a self-review of my code.
  • If it is a core feature, I have added thorough tests.
  • Do we need to implement analytics? if so did you add the relevant metrics to the dashboard?
  • If this PR requires public announcement, mark it with /release-notes label and add several sentences in this section.

Checklist before merging

  • Do not forget to reformat commit message to not include the above checklist

jcsp added 2 commits August 4, 2023 14:48
The overall suite runs comfortably within about 4
minutes when these tests are modified: previously
the runtime is about 10 minutes, most of which is just
spent waiting for these 2 tests to finish.

There's no "right answer" for how long a test should
be, but as a rule of thumb, no one test should run
for much longer than the time it takes to run all the
other tests together.
This was previously at 1000, having been bumped
from an earlier value of 100.

This limited concurrency on systems with larger
numbers of cores, and none of the tests we have
today require more than 384 ports.
@jcsp jcsp added a/test Area: related to testing a/tech_debt Area: related to tech debt labels Aug 4, 2023
@github-actions
Copy link

github-actions bot commented Aug 4, 2023

1264 tests run: 1211 passed, 0 failed, 53 skipped (full report)


@jcsp jcsp marked this pull request as ready for review August 4, 2023 15:20
@vadim2404 vadim2404 requested a review from bayandin August 4, 2023 15:25
@petuhovskiy
Copy link
Member

test_restarts_frequent_checkpoints and similar test_restarts_under_load were useful before, when we had bugs, but now I haven't seen them fail for a long time. +1 for decreasing iterations.

Copy link
Member

@bayandin bayandin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

Don't forget to update "Summary of changes" in the description 😉

@jcsp jcsp merged commit 33cb1e9 into main Aug 8, 2023
@jcsp jcsp deleted the jcsp/faster-tests branch August 8, 2023 08:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a/tech_debt Area: related to tech debt a/test Area: related to testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants